JET + Mooncake fixes for 1.12 #921

penelopeysm · 2025-05-16T11:30:14Z

Mooncake doesn't precompile on 1.12, which was causing CI to fail on julia-version = pre. This removes it from the test suite if the Julia version is a prerelease.

Previously, we knew that CI was failing on 1.12 because of Mooncake. However, removing Mooncake also flagged up a couple more errors that I had to resolve to get CI to pass:

JET.jl has a new version, v0.10, that is intended for Julia 1.12. This PR also includes v0.10 in the allowed compat ranges for JET.jl so that the tests can run on pre. Previously, the JET tests on pre would fail as CI would attempt to install an older version of JET that wasn't forward-compatible. Thankfully, this doesn't require runtime checks because Pkg will automatically figure out the appropriate version of JET to install.
Finally, this PR disables doctests on 1.12, because error messages vary from version to version and it can be very flaky to test correctly for them.

There is one remaining test failure on 1.12, which is related to the use of Threads.threadid() in ThreadSafeVarInfo. I wrote up more about this problem in #924.

Supersedes: #872 #873 #875

github-actions · 2025-05-16T11:33:35Z

Benchmark Report for Commit `86be8b1`

Computer Information

Julia Version 1.11.5
Commit 760b2e5b739 (2025-04-14 06:53 UTC)
Build Info:
  Official https://julialang.org/ release
Platform Info:
  OS: Linux (x86_64-linux-gnu)
  CPU: 4 × AMD EPYC 7763 64-Core Processor
  WORD_SIZE: 64
  LLVM: libLLVM-16.0.6 (ORCJIT, znver3)
Threads: 1 default, 0 interactive, 1 GC (on 4 virtual cores)

Benchmark Results

|                 Model | Dimension |  AD Backend |      VarInfo Type | Linked | Eval Time / Ref Time | AD Time / Eval Time |
|-----------------------|-----------|-------------|-------------------|--------|----------------------|---------------------|
| Simple assume observe |         1 | forwarddiff |             typed |  false |                 10.1 |                 1.5 |
|           Smorgasbord |       201 | forwarddiff |             typed |  false |                734.5 |                36.2 |
|           Smorgasbord |       201 | forwarddiff | simple_namedtuple |   true |                319.3 |                59.8 |
|           Smorgasbord |       201 | forwarddiff |           untyped |   true |               1229.1 |                27.3 |
|           Smorgasbord |       201 | forwarddiff |       simple_dict |   true |               3354.6 |                23.5 |
|           Smorgasbord |       201 | reversediff |             typed |   true |               1470.8 |                30.0 |
|           Smorgasbord |       201 |    mooncake |             typed |   true |                960.9 |                 5.3 |
|    Loop univariate 1k |      1000 |    mooncake |             typed |   true |               5560.3 |                 4.2 |
|       Multivariate 1k |      1000 |    mooncake |             typed |   true |               1027.7 |                 9.5 |
|   Loop univariate 10k |     10000 |    mooncake |             typed |   true |              62719.4 |                 3.7 |
|      Multivariate 10k |     10000 |    mooncake |             typed |   true |               9441.3 |                 9.5 |
|               Dynamic |        10 |    mooncake |             typed |   true |                131.4 |                14.1 |
|              Submodel |         1 |    mooncake |             typed |   true |                 13.5 |                 6.6 |
|                   LDA |        12 | reversediff |             typed |   true |                462.5 |                 5.0 |

github-actions · 2025-05-16T11:40:07Z

DynamicPPL.jl documentation for PR #921 is available at:
https://TuringLang.github.io/DynamicPPL.jl/previews/PR921/

codecov · 2025-05-16T11:47:19Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.89%. Comparing base (072234d) to head (86be8b1).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #921      +/-   ##
==========================================
- Coverage   85.11%   82.89%   -2.23%     
==========================================
  Files          36       36              
  Lines        3956     3957       +1     
==========================================
- Hits         3367     3280      -87     
- Misses        589      677      +88

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

coveralls · 2025-05-16T11:48:25Z

Pull Request Test Coverage Report for Build 15159541807

Details

0 of 0 changed or added relevant lines in 0 files are covered.
1951 unchanged lines in 27 files lost coverage.
Overall coverage decreased (-2.1%) to 83.059%

Files with Coverage Reduction	New Missed Lines	%
ext/DynamicPPLEnzymeCoreExt.jl	1	0.0%
src/extract_priors.jl	5	53.57%
src/test_utils/model_interface.jl	5	22.22%
src/test_utils/varinfo.jl	5	76.19%
src/transforming.jl	5	72.22%
src/logdensityfunction.jl	8	80.85%
src/model_utils.jl	11	0.0%
src/distribution_wrappers.jl	16	0.0%
src/submodel_macro.jl	26	0.0%
src/values_as_in_model.jl	26	0.0%

Totals
Change from base Build 15114067420:	-2.1%
Covered Lines:	3280
Relevant Lines:	3949

💛 - Coveralls

mhauru

Definitely happy with the JET thing, probably happy with the Mooncake thing, unsure about the doctests. Won't we have the same problem of error message variation once the prerelease becomes the next release, and our CI will run both that and min?

mhauru · 2025-05-20T10:31:57Z

test/Project.toml

@@ -29,6 +28,9 @@ StableRNGs = "860ef19b-820b-49d6-a774-d7a799459cd3"
 Test = "8dfed614-e22c-5e08-85e1-65c5234f0b40"
 Zygote = "e88e6eb3-aa80-5325-afca-941959d7151f"

+[weakdeps]


What does it mean for something to be a weak dependency of tests?

To be honest, no idea now, I remember needing to add it to satisfy something. It could have been because of the compat entry i.e. if we remove the Mooncake compat entry from test/Project.toml it might be fine. Let me try that

penelopeysm · 2025-05-20T10:56:53Z

Won't we have the same problem of error message variation once the prerelease becomes the next release, and our CI will run both that and min?

That's true. I contemplated doing it in this PR but didn't — the correct solution IMO is for the doctests to only be built on the current version, and I think the easiest way to do that is to do the doctests as part of the doc building workflow rather than the test workflow. Would you be on board with that?

mhauru · 2025-05-20T11:04:19Z

Hmm, what about a separate workflow just for doctests? Annoying code duplication? They do feel a lot more like tests than docs to me, build-wise.

penelopeysm · 2025-05-20T11:09:54Z

A little bit of workflow yaml code duplication in exchange for not having weird logic inside test/runtests.jl seems like a win to me, let's go with that.

This reverts commit 027ccdc. maxthreadid() causes failures with Mooncake, even on 1.11, so this is not safe to change

penelopeysm · 2025-05-20T15:53:15Z

I ended up reverting the ThreadSafeVarInfo change, because that caused Mooncake to fail on 1.11 (it can't differentiate through maxthreadid()). So we'll have to deal with the Julia 1.12 TSVI failure for now, but at least it's more informative than the previous failure to load the project environment.

mhauru · 2025-05-21T07:57:25Z

test/runtests.jl

-        if !IS_PRERELEASE
-            # Don't run doctests on prerelease as error messages etc. may vary
-            @testset "doctests" begin
-                DocMeta.setdocmeta!(
-                    DynamicPPL,
-                    :DocTestSetup,
-                    :(using DynamicPPL, Distributions);
-                    recursive=true,
-                )
-                doctestfilters = [
-                    # Ignore the source of a warning in the doctest output, since this is dependent on host.
-                    # This is a line that starts with "└ @ " and ends with the line number.
-                    r"└ @ .+:[0-9]+",
-                ]
-                doctest(DynamicPPL; manual=false, doctestfilters=doctestfilters)
-            end
-        end


Could we keep this but have it outside of both Group1 and Group2? I think it would still be good to run it when running tests locally. Didn't think of this earlier, sorry.

Oh, sure, in that case it becomes even easier from the CI part.

mhauru · 2025-05-21T07:59:13Z

docs/Project.toml

@@ -1,4 +1,5 @@
 [deps]
+ADTypes = "47edcb42-4c32-4615-8424-f2b9edc5f35b"


Is this for the doctests? I didn't realise the docs project would need it, but if it's needed then it is.

Indeed, it depends on which env you call doctest() from. In the new workflow I was using the docs env. But I suppose I will move it back to the test env

mhauru

Sorry about the changing requests. Very happy with this now, thanks!

penelopeysm · 2025-05-21T11:20:09Z

No problem, they're good requests 🙂

github-actions bot assigned penelopeysm May 16, 2025

penelopeysm changed the title ~~Don't test Mooncake on 1.12~~ JET + Mooncake fixes for 1.12 May 16, 2025

penelopeysm force-pushed the py/no-mooncake-pre branch 7 times, most recently from c25fd4c to d6e9c83 Compare May 16, 2025 21:38

penelopeysm mentioned this pull request May 16, 2025

ThreadSafeVarInfo and threadid #924

Open

penelopeysm added 6 commits May 18, 2025 18:33

Don't test Mooncake on 1.12

5cab7dd

Bump JET.jl compatibility to include 0.10

32f9054

Disable doctests on prerelease

57bd111

Fix version check

fa479f3

Split prerelease CI into two groups like the rest

16ca002

Use VERSION.prerelease

7363432

penelopeysm force-pushed the py/no-mooncake-pre branch from 625eb18 to 7363432 Compare May 18, 2025 17:33

penelopeysm requested a review from mhauru May 19, 2025 10:09

mhauru reviewed May 20, 2025

View reviewed changes

penelopeysm added 4 commits May 20, 2025 14:36

Remove Mooncake test dep (use DPPL compat)

e0ec601

Move doctests into separate workflow

6974ab2

Merge remote-tracking branch 'origin/main' into py/no-mooncake-pre

3b1a6bc

Fix dependencies needed for doctests

f9d16c7

penelopeysm added 2 commits May 20, 2025 15:15

Enable depwarns for doctests

cac8f9c

Change length of ThreadSafeVarInfo logps to maxthreadid()

027ccdc

penelopeysm changed the title ~~JET + Mooncake fixes for 1.12~~ JET + Mooncake + ThreadSafeVarInfo fixes for 1.12 May 20, 2025

Revert "Change length of ThreadSafeVarInfo logps to maxthreadid()"

af76f81

This reverts commit 027ccdc. maxthreadid() causes failures with Mooncake, even on 1.11, so this is not safe to change

penelopeysm changed the title ~~JET + Mooncake + ThreadSafeVarInfo fixes for 1.12~~ JET + fixes for 1.12 May 20, 2025

penelopeysm changed the title ~~JET + fixes for 1.12~~ JET + Mooncake fixes for 1.12 May 20, 2025

penelopeysm requested a review from mhauru May 20, 2025 15:49

mhauru reviewed May 21, 2025

View reviewed changes

Move doctest invocation back to test/runtests.jl

86be8b1

penelopeysm requested a review from mhauru May 21, 2025 10:48

mhauru approved these changes May 21, 2025

View reviewed changes

penelopeysm added this pull request to the merge queue May 21, 2025

Merged via the queue into main with commit 3acd0a2 May 21, 2025
18 of 20 checks passed

penelopeysm deleted the py/no-mooncake-pre branch May 21, 2025 11:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JET + Mooncake fixes for 1.12 #921

JET + Mooncake fixes for 1.12 #921

penelopeysm commented May 16, 2025 •

edited

Loading

github-actions bot commented May 16, 2025 •

edited

Loading

github-actions bot commented May 16, 2025

codecov bot commented May 16, 2025 •

edited

Loading

coveralls commented May 16, 2025 •

edited

Loading

mhauru left a comment

mhauru May 20, 2025

penelopeysm May 20, 2025

penelopeysm commented May 20, 2025

mhauru commented May 20, 2025

penelopeysm commented May 20, 2025

penelopeysm commented May 20, 2025

mhauru May 21, 2025

penelopeysm May 21, 2025

mhauru May 21, 2025

penelopeysm May 21, 2025

mhauru left a comment

penelopeysm commented May 21, 2025

		@@ -1,4 +1,5 @@
		[deps]
		ADTypes = "47edcb42-4c32-4615-8424-f2b9edc5f35b"

JET + Mooncake fixes for 1.12 #921

JET + Mooncake fixes for 1.12 #921

Conversation

penelopeysm commented May 16, 2025 • edited Loading

github-actions bot commented May 16, 2025 • edited Loading

Benchmark Report for Commit 86be8b1

Computer Information

Benchmark Results

github-actions bot commented May 16, 2025

codecov bot commented May 16, 2025 • edited Loading

Codecov Report

coveralls commented May 16, 2025 • edited Loading

Pull Request Test Coverage Report for Build 15159541807

Details

💛 - Coveralls

mhauru left a comment

Choose a reason for hiding this comment

mhauru May 20, 2025

Choose a reason for hiding this comment

penelopeysm May 20, 2025

Choose a reason for hiding this comment

penelopeysm commented May 20, 2025

mhauru commented May 20, 2025

penelopeysm commented May 20, 2025

penelopeysm commented May 20, 2025

mhauru May 21, 2025

Choose a reason for hiding this comment

penelopeysm May 21, 2025

Choose a reason for hiding this comment

mhauru May 21, 2025

Choose a reason for hiding this comment

penelopeysm May 21, 2025

Choose a reason for hiding this comment

mhauru left a comment

Choose a reason for hiding this comment

penelopeysm commented May 21, 2025

penelopeysm commented May 16, 2025 •

edited

Loading

github-actions bot commented May 16, 2025 •

edited

Loading

Benchmark Report for Commit `86be8b1`

codecov bot commented May 16, 2025 •

edited

Loading

coveralls commented May 16, 2025 •

edited

Loading